Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make networks vms relations distinct #15783

Merged
merged 1 commit into from
Aug 10, 2017

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Aug 10, 2017

Make networks vms relations distinct, otherwise the Vm count
could be shown as higher, if the the vm has multiple ports
plugged into the same network.

Fixes BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1479667

Make networks vms relations distinct, otherwise the Vm count
could be shown as higher, if the the vm has multiple ports
plugged into the same network.

Fixes BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1479667
@Ladas
Copy link
Contributor Author

Ladas commented Aug 10, 2017

before(the VMs counts differ):
screenshot from 2017-08-10 11-18-47
screenshot from 2017-08-10 11-18-39
screenshot from 2017-08-10 11-18-31

after(the VMs counts are the same):
screenshot from 2017-08-10 11-15-12
screenshot from 2017-08-10 11-15-00
screenshot from 2017-08-10 11-14-52

@Ladas
Copy link
Contributor Author

Ladas commented Aug 10, 2017

@miq-bot assign @agrare
@miq-bot add_label bug

@miq-bot
Copy link
Member

miq-bot commented Aug 10, 2017

Checked commit Ladas@77a77d7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find!
Could this break existing spec tests that are expecting network.vms.count to eq(something)?

@Ladas
Copy link
Contributor Author

Ladas commented Aug 10, 2017

@agrare hm, not sure. The specs should test the correct counts now. I think we don't have Vm connected to the same network with 2 ports tested. But seems like it is in the spec data. :-)

@agrare
Copy link
Member

agrare commented Aug 10, 2017

Yeah if the tests fail because of this change they were wrong in the first place :) but we should keep an eye on it and update if any fail after this change.

@agrare agrare merged commit 51c45fa into ManageIQ:master Aug 10, 2017
@agrare agrare added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants